VUU-31 Layout Server POC #39
Conversation
- Temporary change to fix build until screenshot functionality is implemented
#17 Add Save Layout UI Component
… of https://github.com/ScottLogic/finos-vuu into feature/VUU-18-browse-layouts
…ottLogic/finos-vuu into feature/VUU-33-Layout-Manager-Context
…1-save-layout-screenshot VUU21: Implement the screenshot functionality into the Save Layout component
Feature/vuu 18 browse layouts
…ttps://github.com/ScottLogic/finos-vuu into feature/VUU-33-Layout-Manager-Context
…Context VUU-33 add context for layouts
…7-dialogStyling VUU17: Save Layout Panel component styling
vferraro-scottlogic
requested review from
heswell,
pling-scottlogic and
cfisher-scottlogic
| leftSidePanelLayout="full-height" | ||
| leftSidePanel={<LeftNav style={{ width: 240 }} />} | ||
| loginUrl={window.location.toString()} | ||
| user={user} | ||
| saveLocation="local" | ||
| saveUrl="http://127.0.0.1:3000/api/vui/layouts" |
There was a problem hiding this comment.
need to be a little careful with the ShowCase examples. Bear in mind that these are published to CloudFlare pages, where we won't have any server running. It's fine to have stories that require a server - we have plenty that require Vuu. BUT we must handle the scenario where the server isn't available and show an error message.
Also , keep one version of the NewTheme app as a purely standalone example. Ideally, we can have a single example, which can be extended with a second example which provides a saveUrl, so that example would work with the remote REST service
There was a problem hiding this comment.
I agree with your point, but I'm unclear on the best way to go about doing this.
I don't think we should be passing the save location as a prop to the Shell, it feels like something which should sit in an environment/configuration file and pulled from there - with a default location already defined (such as local storage or our server implementation). Does CloudFlare serve local storage? In which case a local storage default would cover the scenario of a server being unavailable.
There was a problem hiding this comment.
Actually, now I'm looking at Peter's PR (#40) and thinking about the problem again, I imagine an implementation which covers these comments will be handled by the interface?
There was a problem hiding this comment.
I think you are right Cara: the saveLocation should be with the LayoutPersistenceManager
| router.get("/", (_, res: Response<Layout[]>) => { | ||
| res.json(layouts); | ||
| }); | ||
|
|
||
| router.get("/:user/latest", (req: Request, res: Response<Layout | string>) => { | ||
| const layout = layouts.find((layout) => layout.metadata.user === req.params.user); | ||
| if (!layout) { | ||
| res.status(404).send("Layout not found"); | ||
| } else { | ||
| res.send(layout); | ||
| } | ||
| }); |
There was a problem hiding this comment.
It would be good to get some kind of contract defined for the layout API. Not as part of this spike but to be captured in a ticket. Particularly as Chris(?) was interested in understanding this server implementation more.
| // mount the LayoutManagement API routes | ||
| app.use('/api/vui/layouts', layoutManagementRoutes); | ||
|
|
There was a problem hiding this comment.
Not majorly important but I think I would rename this file to express-app.tsand use it purely as base configuration for the express app as you've done here. Then I would move this route-specific configuration to /routes/index.ts. Does that make sense?
What would your thoughts be on that?
pling-scottlogic
force-pushed
the
feature/layout-management/layout-management-feature
branch
from
737bed8 to
28180cd
Compare
No description provided.